fix: guard corrupted cache JSON, clamp negative relative times, fix terminalLink tests#1044
Closed
cursor[bot] wants to merge 4 commits into
Closed
fix: guard corrupted cache JSON, clamp negative relative times, fix terminalLink tests#1044cursor[bot] wants to merge 4 commits into
cursor[bot] wants to merge 4 commits into
Conversation
Corrupted cursor_stack data in the pagination_cursors SQLite table causes JSON.parse to throw SyntaxError, crashing all paginated commands (issue list, trace list, dashboard list, explore, etc.). Now wraps the parse in try/catch and validates the result is an array. On corruption, deletes the bad row and returns undefined (cache miss), matching the defensive pattern used in repo-cache.ts. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
…cache getCachedDetection() has three unprotected JSON.parse calls for source_mtimes_json, dir_mtimes_json, and all_dsns_json. Corrupted SQLite data crashes DSN auto-detection, breaking many commands. Wraps all three parses in a single try/catch. On corruption, returns undefined (cache miss) so detection re-runs from scratch, matching the defensive pattern already used in rowToCachedDsnEntry(). Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
… timestamps Clock skew, scheduled/future timestamps, or bad API data cause negative diffMs, producing nonsensical output like '-5m ago'. Clamps diffMs to >= 0 so future dates display as '0m ago' instead of negative values. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
…TTY helper The withTTY helper only saved/restored SENTRY_PLAIN_OUTPUT but not NO_COLOR or FORCE_COLOR. In CI or environments where NO_COLOR=1, isPlainOutput() returns true despite isTTY being set, because NO_COLOR has higher precedence than the TTY check. This caused two terminalLink tests to fail. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
Contributor
|
Contributor
Codecov Results 📊❌ Patch coverage is 57.14%. Project has 4299 uncovered lines. Files with missing lines (2)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.99% 82.03% +0.04%
==========================================
Files 329 329 —
Lines 23883 23917 +34
Branches 15603 15632 +29
==========================================
+ Hits 19581 19618 +37
- Misses 4302 4299 -3
- Partials 1651 1651 —Generated by Codecov Action |
Member
BYK
added a commit
that referenced
this pull request
Jun 3, 2026
…1051) ## Summary Consolidates the five open Cursor BugBot PRs (#908, #947, #973, #1023, #1044) into a single, de-duplicated, rebased PR. Each genuinely-needed fix is verified against current `main`; overlapping fixes use the best variant; stale and subjective changes are dropped. ## Why consolidate The five BugBot PRs heavily overlapped and had gone stale (7–113 commits behind `main`): the cache `JSON.parse` guard appeared 3×, the `withTTY` test fix 2×, the pagination guard 3×. Several also failed CI only on unrelated base drift. This PR lands the real fixes once and adds guardrails so the duplication/staleness doesn't recur. ## Fixes included **Cache hardening** (`fix(db)`) — from #908/#947/#1044 - New `safeParseJson()` db helper; used in `getPaginationState()` (with `Array.isArray` validation) and `getCachedDetection()` so corrupt cached JSON is a cache miss, not a crash. - `log.debug()` added to three silent catch blocks in `sentry-client.ts`. **Event pagination** (`fix(events)`) — from #947 - `listIssueEvents` now uses the shared `autoPaginate()` helper and drops `nextCursor` when a page overshoots `limit`, so `-c next` never skips events. Adds a property test for the `autoPaginate` trim-drops-cursor invariant plus a focused overshoot regression test. **Relative time** (`fix(formatters)`) — from #1044 - `formatRelativeTime` clamps `diffMs` to `>= 0` ("0m ago" instead of "-5m ago"). **Seer** (`fix(seer)`) — from #973/#1023, relates to #958 - `normalizeAgentStatus` maps `canceled`/`cancelled` → `CANCELLED` (fixes a polling-timeout bug) and `need_more_information` → `NEED_MORE_INFORMATION`. - `searchContainersForRootCauses` requires `causes.length > 0` so an empty legacy array no longer blocks the agent-artifact fallthrough. - `extractNoSolutionReason` reads the step-level `description` (current API) before the artifact-level `reason` (legacy). **Release set-commits** (`fix(release)`) — from #1023 - Narrow the default-mode 400 fallback to the exact `NO_REPO_INTEGRATIONS_MESSAGE` so unrelated 400s (invalid refs, bad release state) propagate instead of being masked as "no integration". **Test isolation** (`test`) — from #1044/#947 - `withTTY` now saves/restores `NO_COLOR` and `FORCE_COLOR` (CI sets `NO_COLOR=1`). ## Systemic guardrails - `script/check-error-patterns.ts` gains an advisory silent-catch scan (`SENTRY_STRICT_SILENT_CATCH=1` to enforce). - `AGENTS.md` documents automated-fix-PR rules: check existing PRs/issues first, rebase before review, separate correctness from opinion, prefer shared helpers. - Removed a stale, now-unused `biome-ignore` in `formatters/local.ts` that broke repo-wide lint under biome 2.3.8. ## Dropped (intentionally) - **`issue list` "lifetime" collapse removal** (#973) — superseded on `main` by the `shouldCollapseLifetime` param; issue #969 is already closed. - **Issue selector hint `is:resolved` → bare list** (#1023) — subjective UX; `main`'s current behavior is deliberate. ## Follow-up - A scheduled stale-bot to auto-close inactive bot PRs (out of scope here). ## Testing - `pnpm run typecheck` ✓ - `pnpm run lint` ✓ (766 files) - `pnpm run check:errors` ✓ · `pnpm run check:deps` ✓ - `pnpm run test:unit` ✓ (325 files, 7419 passed, 13 skipped) Closes #908, #947, #973, #1023, #1044.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 4 bugs found via codebase analysis and test suite inspection.
1. Unguarded
JSON.parsein pagination cache crashes paginated commandsFile:
src/lib/db/pagination.tsRoot cause:
getPaginationState()callsJSON.parse(row.cursor_stack)without try/catch. Corrupted SQLite data throwsSyntaxError, crashing all paginated commands (issue list,trace list,dashboard list,explore, etc.).Fix: Wrap in try/catch + validate result is an array. On corruption, delete the bad row and return
undefined(cache miss). Matches the defensive pattern used inrepo-cache.ts.2. Unguarded
JSON.parsein DSN detection cache crashes auto-detectionFile:
src/lib/db/dsn-cache.tsRoot cause:
getCachedDetection()has three unprotectedJSON.parsecalls (source_mtimes_json,dir_mtimes_json,all_dsns_json). Corrupted SQLite data crashes DSN auto-detection, breaking many commands that rely on project detection.Fix: Wrap all three parses in a single try/catch. On corruption, return
undefined(cache miss) so detection re-runs from scratch. Matches the pattern already used inrowToCachedDsnEntry()in the same file.3.
formatRelativeTimeshows negative times for future/skewed timestampsFile:
src/lib/formatters/time-utils.tsRoot cause: Clock skew, scheduled/future timestamps, or bad API data produce negative
diffMs, displayed as-5m ago.Fix: Clamp
diffMsto>= 0so future dates display as0m ago.4.
terminalLinktests fail in CI due to incomplete env save/restoreFile:
test/lib/formatters/colors.test.tsRoot cause: The
withTTY()test helper only saved/restoredSENTRY_PLAIN_OUTPUTbut notNO_COLORorFORCE_COLOR. In CI (whereNO_COLOR=1),isPlainOutput()returns true despiteisTTYbeing set, becauseNO_COLORhas higher precedence.Fix: Save/restore all three env vars (
SENTRY_PLAIN_OUTPUT,NO_COLOR,FORCE_COLOR) in thewithTTYhelper.Test results
terminalLinktests now pass